Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reuse existing license if user didn't set any #104

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

suzaku
Copy link

@suzaku suzaku commented Nov 9, 2023

GitHub now supports creating files like LICENSE for newly created repos.
Running cobra-cli init after cloning a newly created repo will now
empty that existing license.

@CLAassistant
Copy link

CLAassistant commented Nov 9, 2023

CLA assistant check
All committers have signed the CLA.

GitHub now supports creating files like LICENSE for newly created repos.
Running `cobra-cli init` after cloning a newly created repo will now
empty that existing license.
@@ -72,7 +72,17 @@ func (p *Project) createLicenseFile() error {
data := map[string]interface{}{
"copyright": copyrightLine(),
}
licenseFile, err := os.Create(fmt.Sprintf("%s/LICENSE", p.AbsolutePath))

licensePath := p.AbsolutePath + "/LICENSE"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nicer, but the rest of the code use the previous style so this create inconsistency.

It would be better to keep the current style for this change, and if changing the way paths are created is needed, do this in the entire file or project.

}
}

licenseFile, err := os.Create(licensePath)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using exiting license makes sense, but this check ignores errors from os.Stat(), (other then fs.ErrNotExits). If the file exits and os.Stat failed, something is very wrong and failing is the best option.

But I think there is a better way - do we really want to overwrite existing license file if the user specify the license type? This does not feel like a careful way to modify user source tree.

So we can ensure that we never overwrite existing license, and handle the special case when the user did not specify the license, in which we will reuse the existing license.

Something like:

licenseFile, err := os.OpenFile(licensePath, O_CREATE|O_EXCL, 0755)
if err != nil {
    if errors.Is(err, fs.ErrExist) && p.Legal.Name == "None" {
        // User did not specify the license - use the existing one.
        return nil
    }
    return err
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants